Add meta text to DataView #2348
Conversation
jordankoschei-okta
commented
Sep 11, 2024
jordankoschei-okta
commented
| const additionalActions = useMemo(() => { | ||
| return ( | ||
| (metaText || |
There was a problem hiding this comment.
We can make this an implicit return right?
But even then, there's a lot going on in here.
Is there a reason this isn't just another component?
React v19 would make this separate useMemo hook, but we don't need to do it here. It's only boolean checks.
This just seems like an unnecessary optimization by moving the boolean render of these action buttons up here rather than down in the return where we'd expect to see it.
There was a problem hiding this comment.
I like the idea of making this another component, but I don't have time to tackle that right now. Would you be comfortable with us shipping as-is and then doing a fast-follow PR to break this out into a new component?
There was a problem hiding this comment.
Yes because it's more of a code styling thing. It doesn't affect consumers.
OKTA-807412 feat: add metaText to DataView fix: remove unnecessary CSS empty check Merge branch 'main' into jk-table-meta
DES-6504 Add meta text to DataView (#2348) OKTA-807412 feat: add metaText to DataView fix: remove unnecessary CSS empty check Merge branch 'main' into jk-table-meta feat: add onPaginationChange hook Merge branch '1.23' into jk-expose-pagination-change-on-dataview fix: update tests to work in Bacon fix: update tests to work in Bacon fix: update tests to work in Bacon
DES-6516 build: bump versions for 1.23.0 docs: update CHANGELOG for 1.23.0 Add meta text to DataView (#2348) OKTA-807412 feat: add metaText to DataView fix: remove unnecessary CSS empty check Merge branch 'main' into jk-table-meta fix(odyssey-react-mui): remove reference to babel-loader package Add onPaginationChange hook to DataView (#2365) DES-6504 Add meta text to DataView (#2348) OKTA-807412 feat: add metaText to DataView fix: remove unnecessary CSS empty check Merge branch 'main' into jk-table-meta feat: add onPaginationChange hook Merge branch '1.23' into jk-expose-pagination-change-on-dataview fix: update tests to work in Bacon fix: update tests to work in Bacon fix: update tests to work in Bacon fix(odyssey-storybook): trigger test run
